[Bug Fix] Prevent Command dialog stacking#386
Conversation
| this.selectedIndex = -1; | ||
| } | ||
|
|
||
| focusDialogInput(dialog) { |
There was a problem hiding this comment.
it re-implements Stimulus target lookup by hand and hard-codes the controller identifier inside a selector string. If the controller is ever renamed, the input target moves, or the input gets wrapped in another element, this silently stops working, and the controller is now coupled to another instance's internal markup.
Using querySelector is a code smell in hotwire
There was a problem hiding this comment.
Valid point — querySelector + hardcoded selector + manual target lookup couples this controller to another instance's markup.
Refactored to use Stimulus outlets (af84363): split into a new ruby-ui--command-dialog controller that owns the trigger + content target and declares a ruby-ui--command outlet. Outlet connected/disconnected callbacks track the active cloned dialog, and open() calls focusInput() on the outlet instead of querying the DOM. Inner ruby-ui--command now only handles input/items/filter/keydown/dismiss, with no hasContentTarget dual-personality. Marker attribute is now declarative outlet config, not a runtime querySelector.
Replace querySelector + hardcoded selector with a static `openInstance` reference set in connect() and cleared in disconnect(). Removes the `data-ruby-ui--command-dialog` marker attribute and per-controller DOM lookups, so renaming the controller or restructuring dialog markup no longer silently breaks single-instance behavior. Addresses review feedback on #386.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Introduce ruby-ui--command-dialog controller for the trigger/template wrapper and keep ruby-ui--command for the cloned dialog instance. The trigger declares a ruby-ui--command outlet matched by a marker attribute on the cloned dialog wrapper. Outlet connect/disconnect callbacks track the active dialog, replacing the static class field and avoiding both querySelector and same-identifier dual-personality controller code. - New: command_dialog_controller.js (trigger + content target + outlet) - Strip open/openValue/content target from command_controller.js - Rename trigger actions to ruby-ui--command-dialog#open - Add ruby_ui__command_dialog_instance marker on cloned dialog
2 participants
Summary
Closes #230
Testing